Fix never-type rvalue ICE #47746
Fix never-type rvalue ICE #47746
Conversation
src/librustc_mir/build/block.rs
Outdated
| this.cfg.push_assign_unit(block, source_info, destination); | ||
| let tcx = this.hir.tcx(); | ||
| let ty = destination.ty(&this.local_decls, tcx).to_ty(tcx); | ||
| if ty.is_nil() { |
There was a problem hiding this comment.
Just to be sure I understand, the premise here is that the type is either () or else the block must diverge, since any other type would have yielded a compilation error?
There was a problem hiding this comment.
Yeah, with the current semantics, any other type should be impossible (i.e. any other type must be explicit and go through the Some(expr) branch instead) , because the only implicit return types are () and !.
There was a problem hiding this comment.
Could we have a comment? That's a bit non-obvious. (If the code were like if ty.is_never() it'd be more obvious...)
There was a problem hiding this comment.
Sure. (The reason I opted for ty.is_nil() rather than ty.is_never() here is because it seemed less contextual — it's always true that we only want to assign a unit if the type is unit. One alternative to make it absolutely explicit would be to check for !ty.is_never() in an else clause and throw a warning there that it should be impossible, but this seems like overkill.)
kennytm
added
the
S-waiting-on-review
|
@bors r+ |
|
📌 Commit adcb37e has been approved by |
nikomatsakis
added
S-waiting-on-bors
|
⌛ Testing commit adcb37e with merge 9f95bf37e2e7cc237a6c4ee38cdb24a5dda56a52... |
|
💔 Test failed - status-appveyor |
Fix never-type rvalue ICE This fixes #43061. r? @nikomatsakis A small post-mortem as a follow-up to our investigations in #47291: The problem as I understand it is that when `NeverToAny` coercions are made, the expression/statement that is coerced may be enclosed in a block. In our case, the statement `x;` was being transformed to something like: `NeverToAny( {x;} )`. Then, `NeverToAny` is transformed into an expression: https://github.com/rust-lang/rust/blob/000fbbc9b8f88adc6a417f1caef41161f104250f/src/librustc_mir/build/expr/into.rs#L52-L59 Which ends up calling `ast_block_stmts` on the block `{x;}`, which triggers this condition: https://github.com/rust-lang/rust/blob/000fbbc9b8f88adc6a417f1caef41161f104250f/src/librustc_mir/build/block.rs#L141-L147 In our case, there is no return expression, so `push_assign_unit` is called. But the block has already been recorded as _diverging_, meaning the result of the block will be assigned to a location of type `!`, rather than `()`. This causes the MIR error. I'm assuming the `NeverToAny` coercion code is doing what it's supposed to (there don't seem to be any other problems), so fixing the issue simply consists of checking that the destination for the return value actually _is_ supposed to be a unit. (If no return value is given, the only other possible type for the return value is `!`, which can just be ignored, as it will be unreachable anyway.) I checked the other cases of `push_assign_unit`, and it didn't look like they could be affected by the divergence issue (blocks are kind of special-cased in this regard as far as I can tell), so this should be sufficient to fix the issue.
|
☀️ Test successful - status-appveyor, status-travis |
This fixes #43061.
r? @nikomatsakis
A small post-mortem as a follow-up to our investigations in #47291:
The problem as I understand it is that when
NeverToAnycoercions are made, the expression/statement that is coerced may be enclosed in a block. In our case, the statementx;was being transformed to something like:NeverToAny( {x;} ). Then,NeverToAnyis transformed into an expression:rust/src/librustc_mir/build/expr/into.rs
Lines 52 to 59 in 000fbbc
Which ends up calling
ast_block_stmtson the block{x;}, which triggers this condition:rust/src/librustc_mir/build/block.rs
Lines 141 to 147 in 000fbbc
In our case, there is no return expression, so
push_assign_unitis called. But the block has already been recorded as diverging, meaning the result of the block will be assigned to a location of type!, rather than(). This causes the MIR error.I'm assuming the
NeverToAnycoercion code is doing what it's supposed to (there don't seem to be any other problems), so fixing the issue simply consists of checking that the destination for the return value actually is supposed to be a unit. (If no return value is given, the only other possible type for the return value is!, which can just be ignored, as it will be unreachable anyway.)I checked the other cases of
push_assign_unit, and it didn't look like they could be affected by the divergence issue (blocks are kind of special-cased in this regard as far as I can tell), so this should be sufficient to fix the issue.